Skip to content
This repository was archived by the owner on Jan 14, 2026. It is now read-only.

Wrap fileDescriptorRatioGauge which breaks in java9#17

Closed
jhaals wants to merge 3 commits intospotify:masterfrom
jhaals:java9-fix
Closed

Wrap fileDescriptorRatioGauge which breaks in java9#17
jhaals wants to merge 3 commits intospotify:masterfrom
jhaals:java9-fix

Conversation

@jhaals
Copy link
Member

@jhaals jhaals commented Nov 8, 2017

Java9 fails with InaccessibleObjectException which causes all metric
reporting to fail. This is a new exception in java9 so we can't catch it
in Java8, simply return 0 if exception is thrown. This is tested in
java8/java9

java.lang.reflect.InaccessibleObjectException: Unable to make public long com.sun.management.internal.OperatingSystemImpl.getOpenFileDescriptorCount() accessible: module jdk.management does not "opens com.sun.management.internal" to unnamed module @385c9627

jhaals added 3 commits November 8, 2017 15:30
@jhaals
Wrap fileDescriptorRatioGauge which breaks in java9
40be903 
Java9 fails with `InaccessibleObjectException` which causes all metric
reporting to fail. This is a new exception in java9 so we can't catch it
in Java8, simply return 0 if exception is thrown. This is tested in
java8/java9

```
java.lang.reflect.InaccessibleObjectException: Unable to make public long com.sun.management.internal.OperatingSystemImpl.getOpenFileDescriptorCount() accessible: module jdk.management does not "opens com.sun.management.internal" to unnamed module @385c9627
```
@jhaals
fix lint
21c1241
@jhaals
make Exception final
c55d0cd
@udoprog
Copy link
Contributor

udoprog commented Nov 8, 2017

Hey,

Any way this can be detected before we are polling the Gauge, so that we don't have to register the gauge at all if it's not supported?

Maybe try to poll it once during registration?

@jhaals
Copy link
Member Author

jhaals commented Nov 8, 2017

Hi,
Yes that's a good points, we created a quickfix in apollo for this spotify/apollo#231 but we can patch it to not register at all in java9

@udoprog udoprog added the type:enhancement New feature or request label Jan 2, 2018
@udoprog
Copy link
Contributor

udoprog commented Jan 2, 2018

@jhaals proper fix would be appreciated if you have the time :)

@jhaals
Copy link
Member Author

jhaals commented Jan 3, 2018

@udoprog I can wrap the whole gauges.put() in a try catch which would return an empty HashMap on every call to getMetrics, would that be sufficient or do you have anything else in mind?
An alternative could be to throw an exception in the constructor and fail early so that the service using the library can handle it themselves.

@jhaals
Copy link
Member Author

jhaals commented Jan 3, 2018

This was merged into dropwizard/metrics 12 days ago dropwizard/metrics#1236
It won't be out until 4.0 but I guess we can copy their code if you prefer that.

@udoprog
Copy link
Contributor

udoprog commented Jan 3, 2018

Hm, that's an interesting approach. I'd think it would be preferably not to register anything if the operation is not supported.

Apparently we can do an instanceof check instead, that seem to be cleaner than a try-catch?

@mattnworb
Copy link
Member

mattnworb commented Oct 4, 2018

I upgraded the metrics library for exactly this issue in #30, sorry I missed this PR when making that

@jhaals
Copy link
Member Author

jhaals commented Oct 4, 2018

No worries @mattnworb, your PR is preferred 👍

@jhaals jhaals closed this Oct 4, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

No reviews

Assignees

No one assigned

Labels

type:enhancement New feature or request

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@jhaals @udoprog @mattnworb